Skip to content

[222_51]: fix: override tooltip to Toggle cell alignment in tabular context only.#2938

Closed
Prashant27-07 wants to merge 4 commits intoMoganLab:mainfrom
Prashant27-07:fix-toggle-numbering-tooltip
Closed

[222_51]: fix: override tooltip to Toggle cell alignment in tabular context only.#2938
Prashant27-07 wants to merge 4 commits intoMoganLab:mainfrom
Prashant27-07:fix-toggle-numbering-tooltip

Conversation

@Prashant27-07
Copy link
Contributor

the tooltip "Toggle numbering" was misleading - the button actually toggles cell alignment (tabular vs tabular*), not numbering. Changed the tooltip to "Toggle cell alignment" in generic-menu.scm and text-menu.scm.

@Prashant27-07
Copy link
Contributor Author

Fixes #2852

@JackYansongLi
Copy link
Contributor

You should only change the behavior in tabular-like environment. The current change is global so when it comes to the math mode, it becomes:

Screenshot 2026-03-05 at 16 00 42

@JackYansongLi
Copy link
Contributor

Also, documentation and PR number are required.

Prashant27-07 added a commit to Prashant27-07/mogan that referenced this pull request Mar 5, 2026
@Prashant27-07 Prashant27-07 force-pushed the fix-toggle-numbering-tooltip branch from 7c22ae7 to 4abd0e7 Compare March 5, 2026 16:23
Prashant27-07 added a commit to Prashant27-07/mogan that referenced this pull request Mar 5, 2026
@Prashant27-07 Prashant27-07 force-pushed the fix-toggle-numbering-tooltip branch from 4abd0e7 to 53d3326 Compare March 5, 2026 22:04
@Prashant27-07
Copy link
Contributor Author

Hi @JackYansongLi I have updated the fix. The tooltip change is now only applied in tabular context using table-markup-context? in table-menu.scm. The generic-menu.scm and text-menu.scm are reverted back to 'Toggle numbering'. Please review.

@Yuki-Nagori
Copy link
Contributor

@Prashant27-07 Please refer to the guidelines in https://github.com/MoganLab/mogan/blob/main/CONTRIBUTING.md, thanks.

@Prashant27-07 Prashant27-07 force-pushed the fix-toggle-numbering-tooltip branch from 53d3326 to eec4fa9 Compare March 6, 2026 20:08
@Prashant27-07
Copy link
Contributor Author

Updated the commit to [222_51] and added the developer documentation file devel/222_51.md as required by CONTRIBUTING.md. The fix now correctly overrides the tooltip only in tabular context using table-markup-context?.

@JackYansongLi
Copy link
Contributor

please also change the title of this PR.

@Prashant27-07 Prashant27-07 changed the title fix: change tooltip from Toggle numbering to Toggle cell alignment fix: override tooltip to Toggle cell alignment in tabular context only. Mar 7, 2026
@Prashant27-07
Copy link
Contributor Author

@JackYansongLi Done.

@Prashant27-07 Prashant27-07 changed the title fix: override tooltip to Toggle cell alignment in tabular context only. [222_51]: fix: override tooltip to Toggle cell alignment in tabular context only. Mar 8, 2026
@Yuki-Nagori
Copy link
Contributor

Yuki-Nagori commented Mar 9, 2026

@Prashant27-07 Hello, your devel file has a problem; it is conflicting with someone else's.

@Yuki-Nagori
Copy link
Contributor

Then I think that just modifying the text explanation is not very meaningful; the icon should be replaced accordingly. The aligned icon already exists in mogan\TeXmacs\misc\pixmaps\liii\16x16\focus.

@Prashant27-07
Copy link
Contributor Author

@Yuki-Nagori Hi I found the line using tm_numbered.xpm in table-menu.scm. Could you tell me the exact icon filename from liii/16x16/focus that should replace it for the cell alignment button?

@Yuki-Nagori
Copy link
Contributor

Yuki-Nagori commented Mar 9, 2026

@Prashant27-07 I think like tabular and tabular* should be used separately, rather than sharing a function with the numbering switch like before. Then tm_cell_left and tm_cell_center can have a toggle set up. What do you think?

@Prashant27-07
Copy link
Contributor Author

Hi @Yuki-Nagori, when I resolved the conflict in devel/222_51.md, I accepted both changes which merged my content with another person's content into the same file. Should I remove the other person's content and keep only my tooltip fix documentation

@Prashant27-07
Copy link
Contributor Author

Hi @Yuki-Nagori, that sounds like a good approach! Could you guide me a bit on how to implement the toggle setup with tm_cell_left and tm_cell_center? I want to make sure I implement it correctly.

@Yuki-Nagori
Copy link
Contributor

Yuki-Nagori commented Mar 9, 2026

@Prashant27-07 Regarding this issue, I noticed that you used merge instead of rebase. I suggest you use rebase next time.
And I think it's better for you to delete the changes on 222_51.md and then create a new one.

@Yuki-Nagori
Copy link
Contributor

Yuki-Nagori commented Mar 9, 2026

image @Prashant27-07 Search in the code repository for alternate-first-icon and alternate-second-icon, I think it should be similar to it.

@Prashant27-07 Prashant27-07 force-pushed the fix-toggle-numbering-tooltip branch from b81cf3f to 26b2a9c Compare March 9, 2026 13:35
@Prashant27-07
Copy link
Contributor Author

@Yuki-Nagori I've added the alternate icon overrides for tabular context using tm_cell_left.xpm and tm_cell_center.xpm, and cleaned up the devel file. Please review when you get a chance.

@Yuki-Nagori
Copy link
Contributor

@Prashant27-07 Your devel is still written incorrectly. Why not just create a new devel with a new number? Also, your code is clearly wrong. I just sent you a reference example. How can you just use it directly? Besides, you would know it's useless just by testing it. You need to improve your Git knowledge, PR closed.

@Prashant27-07
Copy link
Contributor Author

@Yuki-Nagori I apologize for the messy PR. I'm learning Git and made several mistakes. Could you please share the reference example you mentioned? I'd like to learn from it and do it correctly next time.

@Yuki-Nagori
Copy link
Contributor

@Prashant27-07 I've been quite busy recently, and I'm not very familiar with this part of the code. I think you can start with simpler PRs, and then improve your Git skills. Keep it up!

@Prashant27-07 Prashant27-07 deleted the fix-toggle-numbering-tooltip branch March 10, 2026 16:08
@Prashant27-07
Copy link
Contributor Author

@Yuki-Nagori Thank you for the encouragement! I've already started working on simpler PRs and improving my Git skills. I'll keep it up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants